Repository navigation
fix: remove orphaned FunctionExecutionResultMessage after truncation - #8142
Closed
MOHAMMED WASIM KHAN (wasim-builds) wants to merge 1 commit into
Closed
MOHAMMED WASIM KHAN (wasim-builds) wants to merge 1 commit into
MOHAMMED WASIM KHAN (wasim-builds) wants to merge 1 commit into
Conversation
Previously, TokenLimitedChatCompletionContext only checked index 0 for an orphaned FunctionExecutionResultMessage after truncation. When the middle-index pop removed an AssistantMessage while leaving its paired FunctionExecutionResultMessage elsewhere in the list, it was never removed. Now _remove_orphaned_function_results checks the entire message list, removing any FunctionExecutionResultMessage that doesn't have a matching call_id in an AssistantMessage. Fixes microsoft#7955
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The orphan-removal logic can still retain orphaned results within a multi-result FunctionExecutionResultMessage, and the new test assertion is currently too weak to catch that case.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes a truncation edge case in TokenLimitedChatCompletionContext.get_messages() where removing messages from the middle could leave behind a FunctionExecutionResultMessage without its corresponding FunctionCall, causing some providers (e.g., OpenAI/Anthropic) to reject requests.
Changes:
- Added
_remove_orphaned_function_results()and invoked it after truncation to remove orphaned tool result messages. - Updated imports to support scanning
AssistantMessage/FunctionCallpairs. - Added a regression test reproducing the mid-list orphaning scenario from #7955.
File summaries
| File | Description |
|---|---|
| python/packages/autogen-core/src/autogen_core/model_context/_token_limited_chat_completion_context.py | Adds post-truncation cleanup to remove orphaned function result messages. |
| python/packages/autogen-core/tests/test_model_context.py | Adds a regression test for orphaned function results caused by middle-index truncation. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
6
to
9
| from .._component_config import Component, ComponentModel | ||
| from ..models import ChatCompletionClient, FunctionExecutionResultMessage, LLMMessage | ||
| from .. import FunctionCall | ||
| from ..models import AssistantMessage, ChatCompletionClient, FunctionExecutionResultMessage, LLMMessage | ||
| from ..tools import ToolSchema |
Comment on lines
+95
to
+104
| result: List[LLMMessage] = [] | ||
| for msg in messages: | ||
| if isinstance(msg, FunctionExecutionResultMessage): | ||
| # Keep only if at least one result has a matching call_id | ||
| has_match = any(result.call_id in call_ids for result in msg.content if result.call_id) | ||
| if has_match: | ||
| result.append(msg) | ||
| else: | ||
| result.append(msg) | ||
| return result |
Comment on lines
+260
to
+261
| assert any(r.call_id in call_ids for r in msg.content), \ | ||
| "Orphaned FunctionExecutionResultMessage found without matching AssistantMessage" |
Author
|
Superseded by #8183 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #7955
Previously, TokenLimitedChatCompletionContext only checked index 0 for an orphaned FunctionExecutionResultMessage after truncation. When the middle-index pop removed an AssistantMessage while leaving its paired FunctionExecutionResultMessage elsewhere in the list, it was never removed. This caused providers like OpenAI and Anthropic to reject requests with a 400 error.
Changes
Test plan